Skip to content

Refactor DPIUtil:autoScaleImageData for better design #2048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 24, 2025
Merged

Refactor DPIUtil:autoScaleImageData for better design #2048

merged 1 commit into from
Apr 24, 2025
+69 −41

Conversation

@amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Apr 22, 2025 •
edited
Loading

This PR contributes to refactoring the logic for scaling the ImageData using GC from DPIUtil to Image for each platform, where it belongs better and DPIUtil calls the platform specific code from the method DPIUtil:autoScaleImageData.

contributes to #62 and #127

@amartya4256 amartya4256 marked this pull request as draft April 22, 2025 14:42
@github-actions
Copy link
Contributor

github-actions bot commented Apr 22, 2025 •
edited
Loading

Test Results

   545 files  + 6     545 suites  +6   32m 9s ⏱️ - 1m 10s
 4 373 tests +37   4 355 ✅ +35   18 💤 +3  0 ❌  - 1 
16 634 runs  +37  16 496 ✅ +35  138 💤 +3  0 ❌  - 1 

Results for commit 215cbfd. ± Comparison against base commit 033d733.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the feature/refactor_imageData_scaling branch 3 times, most recently from ff48400 to 6ff17bd Compare April 23, 2025 08:59
@amartya4256 amartya4256 marked this pull request as ready for review April 23, 2025 08:59
@amartya4256 amartya4256 changed the title WIP Refactor DPIUtil:autoScaleImageData for better design Apr 23, 2025
@amartya4256 amartya4256 mentioned this pull request Apr 23, 2025
Closed
@amartya4256 amartya4256 linked an issue Apr 23, 2025 that may be closed by this pull request
Move scaling ImageData implementation to Image for all platforms vi-eclipse/Eclipse-Platform#273
Closed
@amartya4256 amartya4256 force-pushed the feature/refactor_imageData_scaling branch from 6ff17bd to bce0582 Compare April 24, 2025 08:16
@amartya4256 @HeikoKlare
Move ImageData scaling logic to Image eclipse-platform#62
215cbfd 
This commit contributes to moving the logic for scaling the ImageData
using GC from DPIUtil to Image for each platform and DPIUtil calls the
platform specific code from the method DPIUtil:autoScaleImageData.

contributes to
eclipse-platform#62 and
eclipse-platform#127
@HeikoKlare HeikoKlare force-pushed the feature/refactor_imageData_scaling branch from bce0582 to 215cbfd Compare April 24, 2025 15:56
@HeikoKlare
Copy link
Contributor

HeikoKlare commented Apr 24, 2025

Version increment check is failing for infrastructure reasons and no further version bump is required here.

@HeikoKlare HeikoKlare merged commit 039d965 into eclipse-platform:master Apr 24, 2025
9 of 10 checks passed
@HeikoKlare HeikoKlare deleted the feature/refactor_imageData_scaling branch April 24, 2025 16:11
@HannesWell
Copy link
Member

HannesWell commented Apr 24, 2025

Sorry for joining this discussion (too) late, but I think adding more 'internal' methods to a public API class, especially such an important one like Image, even if they are marked as @noreference, really should be avoided as much as possible.

In this case I think it would be better to add it to a platform-specific util class.
My first idea would be renaming NativeImageLoader to something like NativeImage and add it there. But better ideas are welcome.

@HeikoKlare
Copy link
Contributor

HeikoKlare commented Apr 25, 2025

In this case I think it would be better to add it to a platform-specific util class.
My first idea would be renaming NativeImageLoader to something like NativeImage and add it there. But better ideas are welcome.

Sounds good to me 👍 So it would basically become an internal utility class for everything related to OS-specific image?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akoch-yatta akoch-yatta akoch-yatta left review comments

@HeikoKlare HeikoKlare HeikoKlare approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Move scaling ImageData implementation to Image for all platforms

4 participants

@amartya4256 @HeikoKlare @HannesWell @akoch-yatta